Conversation
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
TextInput中的integerOnly分支会在每次变更时重写整个字符串,这对用户来说可能会有些突兀(光标跳动、粘贴时出现意料之外的转换);建议将校验/规范化逻辑放到onBlur中,或使用一个辅助函数,在字符串实际无效之前尽量避免修改它。- 当
type='number'且integerOnly为 true 时,组件仍然允许像'-'或''这样的过渡状态值,这些值无法很好地映射到原生 number 输入类型上,可能会导致一些奇怪的浏览器行为;在这种场景下,将type保持为'text'并使用inputMode='numeric'可能更安全,只在使用方层面再将其转换为数字。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `integerOnly` branch in `TextInput` rewrites the entire string on every change, which can be jarring for users (caret jumps, unexpected transformations on paste); consider delegating validation/normalization to `onBlur` or a helper that avoids changing the string unless actually invalid.
- When `type='number'` and `integerOnly` is true, the component still allows transient values like `'-'` or `''`, which don’t map cleanly to native number inputs and may cause odd browser behavior; it may be safer to keep `type='text'` with `inputMode='numeric'` for this path and only coerce to a number at the consumer level.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的代码评审。
Original comment in English
Hey - I've left some high level feedback:
- The
integerOnlybranch inTextInputrewrites the entire string on every change, which can be jarring for users (caret jumps, unexpected transformations on paste); consider delegating validation/normalization toonBluror a helper that avoids changing the string unless actually invalid. - When
type='number'andintegerOnlyis true, the component still allows transient values like'-'or'', which don’t map cleanly to native number inputs and may cause odd browser behavior; it may be safer to keeptype='text'withinputMode='numeric'for this path and only coerce to a number at the consumer level.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `integerOnly` branch in `TextInput` rewrites the entire string on every change, which can be jarring for users (caret jumps, unexpected transformations on paste); consider delegating validation/normalization to `onBlur` or a helper that avoids changing the string unless actually invalid.
- When `type='number'` and `integerOnly` is true, the component still allows transient values like `'-'` or `''`, which don’t map cleanly to native number inputs and may cause odd browser behavior; it may be safer to keep `type='text'` with `inputMode='numeric'` for this path and only coerce to a number at the consumer level.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This pull request adds number input support for integer pipeline types in the option editor. The implementation allows users to input integer values with proper validation and mobile-optimized keyboard layouts.
Changes:
- Enhanced TextInput component with number input support including integer-only validation that strips non-numeric characters except minus signs
- Configured OptionEditor to automatically use number input mode for inputs with
pipeline_type: 'int' - Added
interface.jsonto.gitignoreto exclude generated MaaFramework interface files
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/FormControls.tsx | Added type, inputMode, step, and integerOnly props to TextInput component with custom onChange handler for integer validation |
| src/components/OptionEditor.tsx | Conditionally passes number input props to TextInput when pipeline_type === 'int' |
| .gitignore | Excludes generated interface.json file from version control |
| if (raw === '' || raw === '-') { | ||
| onChange(raw); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The integer input validation allows the user to enter just a minus sign ("-"), which will be passed to the pipeline override generation. In pipelineOverride.ts line 69, when inputVal is "-", the expression inputVal || '0' evaluates to "-" (since it's truthy), resulting in invalid JSON when injected into the pipeline override. Consider changing the early return condition to only allow empty string, or parse and validate the value before allowing it through. For example: if (raw === '') { onChange(raw); return; } and handle the minus sign case by preventing it or treating it as '0' or '-0'.
| if (raw === '' || raw === '-') { | |
| onChange(raw); | |
| return; | |
| } | |
| if (raw === '') { | |
| onChange(raw); | |
| return; | |
| } | |
| if (raw === '-') { | |
| // Prevent a bare minus sign from being stored as the value. | |
| // This avoids propagating "-" into downstream JSON generation. | |
| return; | |
| } |
Summary by Sourcery
为共享的 TextInput 组件添加仅限整数验证的数值输入支持,并将其接入用于整数类型流水线字段的选项编辑器。
New Features:
inputMode和step属性的数字字段。Original summary in English
Summary by Sourcery
Add support for numeric inputs with integer-only validation to the shared TextInput component and wire it into the option editor for integer pipeline fields.
New Features: